Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding snippets directory and all required symbolic links #39854

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

mjpytlak
Copy link
Contributor

@mjpytlak mjpytlak commented Dec 14, 2021

This PR is a follow on effort to enabling the use of text snippets. While PR 38393 updated the OpenShift documentation guidelines on how to use text snippets. This PR prepares the repository for their adoption.

Material changes.

  1. technology-preview.adoc was moved from the top-level modules directory to a new top-level snippets directory.
  2. All existing topics that include technology-preview.adoc have been updated to reference the new location. All files where tested locally and look good.
  3. A symbolic link to the top-level snippets directory has been added to all remaining content directories, enabling the use of text snippets.

Doc preview of topics using technology-preview.adoc

@netlify
Copy link

netlify bot commented Dec 14, 2021

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 2e9b9d8

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/61fadd697402d80007075a8e

😎 Browse the preview: https://deploy-preview-39854--osdocs.netlify.app

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 14, 2021
@mjpytlak
Copy link
Contributor Author

@vikram-redhat Given the nature of this change, I am asking that you peer review this. I was thinking that it might be a good idea to begin adoption of text snippets with 4.10 to avoid any issues that might arise going as far back as 4.6. WDYT?

@vikram-redhat
Copy link
Contributor

@mjpytlak this works for me.

Do you mean to say that you will merge this only for 4.10?

@mjpytlak
Copy link
Contributor Author

mjpytlak commented Dec 16, 2021

Yes. I am proposing that we adopt text snippets as of 4.10. I was concerned about back porting and running into issues. Am I overthinking it?

@jboxman-rh
Copy link

@vikram-redhat, this doesn't count as a module in the CCS guidelines and is thus permissible? Or are we not concerned about that any longer. For all intents in purposes, this is just structured use of an include. And if we were just docs.openshift.com, I'd say go for it. But because we have other concerns, I wonder about this if adopted at scale.

@mjpytlak
Copy link
Contributor Author

Good morning, @jboxman-rh. If you have not had a chance, PTAL at #38393. I had updated our guidelines around usage.

@vikram-redhat
Copy link
Contributor

@jboxman I don't think there are any issues around using text snippets, in any platform. @kalexand-rh can you confirm if Jupiter has any restrictions on this?

@mjpytlak if we do merge it, I think we should backport it to avoid conflicts.

@jboxman
Copy link
Contributor

jboxman commented Dec 16, 2021

@vikram-redhat, well, a snippet is just an include. So this all formalizes what to Asciidoctor is just a thing, that it does. But it isn't necessarily a thing every 'platform' will do, so I don't want to keep digging if it presents an issue.

@kalexand-rh
Copy link
Contributor

@vikram-redhat, Jupiter doesn't care if you use snippets. It's just another include. I don't think they've decided if they're going to store it as a separate node, but they've confirmed that snippets will be transformed correctly during the ingest phase.

@vikram-redhat
Copy link
Contributor

well, a snippet is just an include. So this all formalizes what to Asciidoctor is just a thing, that it does. But it isn't necessarily a thing every 'platform' will do, so I don't want to keep digging if it presents an issue.

You are right. We know how it can be with other "platforms" :).

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 25, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 26, 2022
@mjpytlak mjpytlak changed the title Adding snippets directory and all required symblic links Adding snippets directory and all required symbolic links Jan 26, 2022
@mjpytlak
Copy link
Contributor Author

@kalexand-rh Rebased and resolved references to modules/technology-preview.adoc Based on Vikram's comments, I believe he was suggesting this be back ported, but he did not say how far back. I am presuming he meant all the way back to 4.6.

@kalexand-rh
Copy link
Contributor

@mjpytlak, moving this back to 4.6 will be best.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2022
@kalexand-rh kalexand-rh added this to the Next Release milestone Feb 2, 2022
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2022
@kalexand-rh kalexand-rh merged commit 9130866 into openshift:main Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants